Skip to content

Update Docs for DateTime.Ticks to explain it depends on the time zone of the object instance. #3355

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jan 21, 2020

Conversation

AWilco
Copy link
Contributor

@AWilco AWilco commented Oct 16, 2019

Summary

Corrected documentation of Ticks property to include information that it is related to the number of ticks since 0:00:00 on January 1, 0001 in the time zone of the the DateTime instance, and not always UTC. Thus using ToUniversalTime() and ToLocalTime() can change the number of ticks, even though it represents the same instant in time.

As discussed here: https://social.msdn.microsoft.com/Forums/vstudio/en-US/fde7e5b0-e2b9-4d3b-8a63-c2ae75e316d8/datetimeticks-bug?forum=netfxbcl
And tested on .NET 4.5.1.

If this has changed in the latest .NET runtimes, the documentation says that it has not and is the same functionality as 4.5.1

Not known to be an open issue.

Corrected documentation of Ticks property to include information that it is related to the number of ticks since 0:00:00 on January 1, 0001 in the time zone of the the DateTime instance, and not UTC. Thus using ToUniversalTime() and ToLocalTime() can change the number of ticks, even though it represents the same instance of time.

As documented here: https://social.msdn.microsoft.com/Forums/vstudio/en-US/fde7e5b0-e2b9-4d3b-8a63-c2ae75e316d8/datetimeticks-bug?forum=netfxbcl
And tested on .NET 4.5.1.

If this has changed in the latest .NET runtimes, the documentation says that it has not and is the same functionality as 4.5.1
@AWilco AWilco changed the title Update DateTime.xml Update Docs for DateTime.Ticks to explain it depends on the time zone of the object instance. Oct 16, 2019
@Thraka
Copy link
Contributor

Thraka commented Oct 17, 2019

@AWilco Thanks a bunch for correcting this. I'll get someone to double check it. 👍

@Thraka Thraka added the waiting-on-reviews Indicates PRs that cannot be merged because of the lack of reviews label Oct 17, 2019
@Thraka Thraka added this to the October 2019 milestone Oct 17, 2019
@Thraka Thraka added the ✨ 1st-time dotnet-api-docs contributor! Indicates PRs from new contributors to the dotnet-api-docs repository label Oct 17, 2019
@Thraka Thraka requested a review from tarekgh October 17, 2019 20:47
@AWilco
Copy link
Contributor Author

AWilco commented Oct 18, 2019

@tarekgh I appreciate that what I have written might not be the factual truth of how DateTime works.

What I intended to convey was that the Ticks property is NOT always the number of ticks since 0:00:00 UTC on January 1, 0001. As I understand it, given limited testing myself and reviewing the discussion thread I linked to, if a timezone is set then the Ticks property is the number of ticks since 0:00:00 on January 1, 0001 in that timezone.

Practically what I was attempting to solve was that when I read that description, I then assume that if I take a specific instant in time represented in a DateTime Object, e.g. 2019-10-01T00:00:00UTC
and change the timezone used to represent that instant, e.g. to
2019-10-01T01:00:00UTC+1

Then the Ticks property would stay the same, as the elapsed time between either of those dates and 0:00:00 UTC on January 1, 0001. However this is not true.

As a test:

            var dateInUtc = new DateTime(1, 1, 1, 1, 0, 0, DateTimeKind.Utc);
            var ticksInUtc = dateInUtc.Ticks;
            var dateInLocal = dateInUtc.ToLocalTime(); //My local time zone is London time, as of writing it is UTC+1
            var ticksInLocal = dateInLocal.Ticks;
            Assert(ticksInUtc - ticksInLocal == 0); //This fails

My new description may not be 100% accurate, I don't know enough about DateTime to ensure what I write is fully accurate, I would appreciate help in improving my new text.

Regarding what happens if you use DateTime.Unspecified, converting to UTC or to local appears to avoid adjusting the ticks at all, and just changes the DateTimeKind property. This means that the instant of time that is represented may be different, but the Ticks property will stay the same.

@tarekgh
Copy link
Member

tarekgh commented Oct 18, 2019

@AWilco Thanks for tying to clarify the docs. what about the following? do you think it is good enough or we need to specify more clarification?

The value of this property represents the number of 100-nanosecond intervals that have elapsed since 12:00:00 midnight, January 1, 0001 in the Gregorian calendar, which represents DateTime.MinValue. It does not include the number of ticks that are attributable to leap seconds.
If the DateTime object has Kind property set to Local, that means the ticks inside this object is representing the time elapsed time since 12:00:00 midnight, January 1, 0001 in the local time specified by the current time zone setting.
If the DateTime object has Kind property set to Utc, that means the ticks inside this object is representing the time elapsed time since 12:00:00 midnight, January 1, 0001 in the Coordinated Universal Time.
If the DateTime object has Kind property set to Unspecified, that means the ticks inside this object is representing the time elapsed time since 12:00:00 midnight, January 1, 0001 in some time zone time which not necessary match Local not Utc time.

In general, the ticks represent the time according to the time zone specified by the Kind property.

@AWilco
Copy link
Contributor Author

AWilco commented Oct 25, 2019

@AWilco Thanks for tying to clarify the docs. what about the following? do you think it is good enough or we need to specify more clarification?

The value of this property represents the number of 100-nanosecond intervals that have elapsed since 12:00:00 midnight, January 1, 0001 in the Gregorian calendar, which represents DateTime.MinValue. It does not include the number of ticks that are attributable to leap seconds.
If the DateTime object has Kind property set to Local, that means the ticks inside this object is representing the time elapsed time since 12:00:00 midnight, January 1, 0001 in the local time specified by the current time zone setting.
If the DateTime object has Kind property set to Utc, that means the ticks inside this object is representing the time elapsed time since 12:00:00 midnight, January 1, 0001 in the Coordinated Universal Time.
If the DateTime object has Kind property set to Unspecified, that means the ticks inside this object is representing the time elapsed time since 12:00:00 midnight, January 1, 0001 **in the same unspecified time zone**

In general, the ticks represent the time according to the time zone specified by the Kind property.

HI Tarek,

Yes, I think what you have is good, although your words about an unspecified time zone seemed to suggest it would be random. In actuality, if the timezone is unspecified, then the whole issue is moot, because there isn't any way to take into account a time zone. The starting instant of 12:00:00 midnight, January 1, 0001 is in the same unspecified time zone. You could aso say that it assumes the time is in UTC for the purposes of this property.

If the DateTime object has Kind property set to Unspecified, that means the ticks inside this object is representing the time elapsed time since 12:00:00 midnight, January 1, 0001 **in the same unspecified time zone**

Either way, what you have proposed is an improvement and doesn't introduce any new inaccuracies.

@Thraka
Copy link
Contributor

Thraka commented Oct 25, 2019

@AWilco let me know when you make the changes with the new suggestions. Thanks.

@Thraka Thraka added assigned-to-author Assigned to the author to resolve and removed waiting-on-reviews Indicates PRs that cannot be merged because of the lack of reviews assigned-to-author Assigned to the author to resolve labels Oct 25, 2019
@tarekgh
Copy link
Member

tarekgh commented Oct 25, 2019

@AWilco thanks for the feedback.

You could aso say that it assumes the time is in UTC for the purposes of this property.

We cannot be specific about this assumption here. There are some contexts that unspecified treated as UTC and in other contexts treated as local.
does it make sense if we changed in the same unspecified time zone to in the unknown time zone?

Feel free to suggest something different. My goal is to have anyone reading to understand how to use it. thanks again.

@carlossanlop
Copy link
Contributor

@AWilco do you have an update on the feedback received on this PR?

Incorporated feedback from review.
@AWilco
Copy link
Contributor Author

AWilco commented Dec 12, 2019

@tarekgh @carlossanlop I've updated the change with the new text as discussed.

@carlossanlop carlossanlop requested a review from tarekgh December 12, 2019 16:59
@carlossanlop carlossanlop added the changes-addressed Indicates PRs that had all comments addressed and are awaiting for new review label Dec 12, 2019
Copy link
Contributor

@carlossanlop carlossanlop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some minor suggestions for you to consider, @AWilco . Thanks for making these changes.

@Thraka, @mairaw, @gewarren it would be awesome if you could double check my language review.

Copy link
Contributor

@gewarren gewarren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a couple small tweaks to Carlos' suggestions.

AWilco and others added 4 commits December 19, 2019 17:41
Co-Authored-By: Genevieve Warren <[email protected]>
Co-Authored-By: Genevieve Warren <[email protected]>
Co-Authored-By: Carlos Sanchez Lopez <[email protected]>
Co-Authored-By: Carlos Sanchez Lopez <[email protected]>
@AWilco
Copy link
Contributor Author

AWilco commented Dec 19, 2019

Thanks, I believe I have accepted all the proposed changes.

Co-Authored-By: Genevieve Warren <[email protected]>
@Thraka Thraka closed this Jan 21, 2020
@Thraka Thraka reopened this Jan 21, 2020
@Thraka Thraka dismissed carlossanlop’s stale review January 21, 2020 19:51

Changes were completed

@Thraka
Copy link
Contributor

Thraka commented Jan 21, 2020

@AWilco Awesome! Thanks a bunch for the updates.

@Thraka Thraka merged commit afc9aa9 into dotnet:master Jan 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.DateTime changes-addressed Indicates PRs that had all comments addressed and are awaiting for new review ✨ 1st-time dotnet-api-docs contributor! Indicates PRs from new contributors to the dotnet-api-docs repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants